kimik2.5-fp4-b200-vllm: expand concurrency sweep to 1-128 - #1677
Conversation
TP4/EP1 conc 4-64 -> 1-128 and TP8/EP1 conc-start 4 -> 1 for the 1k/1k and 8k/1k cells. Appends a perf-changelog entry.
|
Thanks for the contribution! For vLLM & SGLang, please ensure that your recipes is similar to the official vLLM recipes and/or the SGLang cookbook If it is not, please create a PR first before we can merge your single node PR into the master branch. Let's ensure that the documentation is first class such that the entire ML community can benefit from your hard work! Thank you
PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. If re-running failed jobs is attempted, PR authors are responsible for ensuring it passes. See GitHub's docs on re-running failed jobs: https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs#re-running-failed-jobs-in-a-workflow As a rule of thumb, generally, PR authors should request a review & get a PR approval from the respective companies' CODEOWNERS before requesting a review from core maintainers. If additional help is needed, PR authors can reach out to core maintainers over Slack. |
2 similar comments
|
Thanks for the contribution! For vLLM & SGLang, please ensure that your recipes is similar to the official vLLM recipes and/or the SGLang cookbook If it is not, please create a PR first before we can merge your single node PR into the master branch. Let's ensure that the documentation is first class such that the entire ML community can benefit from your hard work! Thank you
PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. If re-running failed jobs is attempted, PR authors are responsible for ensuring it passes. See GitHub's docs on re-running failed jobs: https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs#re-running-failed-jobs-in-a-workflow As a rule of thumb, generally, PR authors should request a review & get a PR approval from the respective companies' CODEOWNERS before requesting a review from core maintainers. If additional help is needed, PR authors can reach out to core maintainers over Slack. |
|
Thanks for the contribution! For vLLM & SGLang, please ensure that your recipes is similar to the official vLLM recipes and/or the SGLang cookbook If it is not, please create a PR first before we can merge your single node PR into the master branch. Let's ensure that the documentation is first class such that the entire ML community can benefit from your hard work! Thank you
PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. If re-running failed jobs is attempted, PR authors are responsible for ensuring it passes. See GitHub's docs on re-running failed jobs: https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs#re-running-failed-jobs-in-a-workflow As a rule of thumb, generally, PR authors should request a review & get a PR approval from the respective companies' CODEOWNERS before requesting a review from core maintainers. If additional help is needed, PR authors can reach out to core maintainers over Slack. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| - kimik2.5-fp4-b200-vllm | ||
| description: | ||
| - "Expand concurrency sweep for the 1k/1k and 8k/1k cells: TP4/EP1 conc 4-64 -> 1-128, TP8/EP1 conc-start 4 -> 1 (conc-end 4 unchanged)." | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1677 |
There was a problem hiding this comment.
🔴 The new perf-changelog.yaml entry has pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/<TODO> instead of /pull/1677. The literal <TODO> produces a broken URL (encoded as %3CTODO%3E, 404s) and breaks any downstream tooling that parses pr-link as a numeric PR id. Replace <TODO> with 1677 before merging — the PR's own Cursor Bugbot summary already flags this placeholder.
Extended reasoning...
What the bug is
At perf-changelog.yaml:3509 the newly appended entry contains:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/<TODO>The string <TODO> is a literal placeholder that was never replaced with the real PR number (1677). I verified this by running git show HEAD -- perf-changelog.yaml on commit bfa691a (the PR's tip) — the added line in the commit itself reads pull/<TODO>, not pull/1677. The PR description's own embedded Cursor Bugbot summary calls this out explicitly: "PR link still has a <TODO> placeholder".
Addressing the refutation
One verifier argued the PR diff shows 1677 and is authoritative over the local checkout. I re-checked directly: git show bfa691a -- perf-changelog.yaml (the PR's tip commit) shows the added line as pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/<TODO>. The file in the working tree at perf-changelog.yaml:3509 also contains <TODO>. Whatever diff rendering showed 1677 does not match the actual committed contents on this branch. If the PR is merged as-is from bfa691a, <TODO> lands in main.
Impact
- Broken URL. GitHub URL-encodes
<and>as%3C/%3E, so the rendered link becomeshttps://github.com/SemiAnalysisAI/InferenceX/pull/%3CTODO%3E, which 404s. - Downstream tooling. Every other entry in
perf-changelog.yamluses a real integer PR id (e.g.1668,1669,1670just above,1544, etc.). Any consumer that parses the trailing path component as an int (int(url.rsplit('/', 1)[-1])) will throwValueErroron this row. - Data integrity for the changelog itself — the file is the canonical record of perf-config changes and should be self-consistent.
Step-by-step proof
$ git rev-parse HEAD
bfa691ad40dbc3074e00073d9afdaf1f68b37c18
$ sed -n '3505,3510p' perf-changelog.yaml
- config-keys:
- kimik2.5-fp4-b200-vllm
description:
- "Expand concurrency sweep for the 1k/1k and 8k/1k cells: ..."
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/<TODO>
$ git show HEAD -- perf-changelog.yaml | grep pr-link
+ pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/<TODO>
Fix
One-character cleanup: replace <TODO> with 1677 on line 3509. The PR number is already known (this PR is #1677, as referenced throughout the description).
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=27046359129 |
|
/reuse-sweep-run |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=27077703341 |
2 participants
Expand the concurrency sweep for
kimik2.5-fp4-b200-vllmacross both the 1k/1k and 8k/1k cells:conc 4-64→1-128conc-start 4→1(conc-end 4unchanged)Appends a perf-changelog entry.
Note
Low Risk
Benchmark YAML and changelog only; no runtime, auth, or application logic changes.
Overview
Widens the fixed-seq-len perf search space for
kimik2.5-fp4-b200-vllmon B200 vLLM for both 1k/1k and 8k/1k cells.For TP4/EP1, concurrency is swept from 4–64 to 1–128. For TP8/EP1, conc-start moves from 4 to 1 while conc-end stays 4. A matching perf-changelog entry is added for
kimik2.5-fp4-b200-vllm.Reviewed by Cursor Bugbot for commit 37e243c. Bugbot is set up for automated code reviews on this repo. Configure here.